Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tag Processor: Add ability to stop at tag closers, if requested #45789

Merged
merged 1 commit into from
Nov 23, 2022

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Nov 15, 2022

Notes

What?

Add an option to the WP_HTML_Tag_Processor query to indicate that the processor should stop and visit tag closers while walking through the input document.

Why?

WP_HTML_Tag_Processor introduces the ability to walk through an HTML document and modify attributes on tag openers.

At times it could be useful to stop also at tag closers in order to perform augmented inspection and querying of a document.

In this patch we're opening up a mode that allows just that; if querying with the [ "tag_closers" => "visit" ] query parrameter then in addition to stopping at tag openers, this will allow stopping at all tag tokens regardless of open or close status.

How?

Introduces internal mechanics to detect and stop at tag closers.
Exposes new query option "tag_closers" => "visit" | "skip" to pass into next_tag() which indicates whether or not to stop at tag closers.
Exposes new is_tag_closer() method to determine if current tag is an opener or closer (from calling code).

Testing Instructions

Review the new unit tests to confirm behavior.

@codesandbox
Copy link

codesandbox bot commented Nov 15, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@@ -408,7 +439,16 @@ public function next_tag( $query = null ) {
return false;
}

$this->parse_tag_opener_attributes();
while ( $this->parse_next_attribute() ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the inlining

Comment on lines 1154 to 1203
* Example:
* <code>
* $p = new WP_HTML_Tag_Processor( '<div></div>' );
* $p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] );
* $p->is_tag_closer() === false;
*
* $p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] );
* $p->iss_tag_closer() === true;
* </code>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice example! A minor typo:

Suggested change
* Example:
* <code>
* $p = new WP_HTML_Tag_Processor( '<div></div>' );
* $p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] );
* $p->is_tag_closer() === false;
*
* $p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] );
* $p->iss_tag_closer() === true;
* </code>
* Example:
* <code>
* $p = new WP_HTML_Tag_Processor( '<div></div>' );
* $p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] );
* $p->is_tag_closer() === false;
*
* $p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] );
* $p->is_tag_closer() === true;
* </code>

$p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] );
$this->assertFalse( $p->is_tag_closer(), 'Indicated a tag opener is a tag closer' );
$this->assertTrue( $p->next_tag( [ 'tag_name' => 'div', 'tag_closers' => 'visit' ] ), 'Did not stop at desired tag closer' );
$this->assertTrue( $p->is_tag_closer(), 'Indicated a tag closer is a tag opener' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see some tests to confirm the behavior of the other methods in the tag closer state, e.g. update_attribute(), __toString(), etc

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a test to confirm that ops are inert inside closed tags

@adamziel
Copy link
Contributor

This looks good @dmsnell! There are some conflicts and I left a few comments, but the direction is great.

@dmsnell dmsnell force-pushed the tag-processor/stop-at-tag-closers branch 2 times, most recently from 46e906e to 1652965 Compare November 17, 2022 05:04
Copy link
Contributor

@adamziel adamziel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the updates @dmsnell! The linter fails, but the tests are passing. This is a solid PR that makes a great foundation for the upcoming unsafe mode. 🚢

@dmsnell dmsnell force-pushed the tag-processor/stop-at-tag-closers branch 2 times, most recently from 5e96d70 to 8bc31a3 Compare November 22, 2022 20:57
@dmsnell
Copy link
Member Author

dmsnell commented Nov 22, 2022

Fixed the lint issues

@dmsnell dmsnell force-pushed the tag-processor/stop-at-tag-closers branch from 8bc31a3 to 0d78f1d Compare November 22, 2022 21:30
`WP_HTML_Tag_Processor` introduces the ability to walk through an HTML
document and modify attributes on tag openers.

At times it could be useful to stop also at tag closers in order to
perform augmented inspection and querying of a document.

In this patch we're opening up a mode that allows just that; if
querying with the `[ "tag_closers" => "visit" ]` query parrameter
then in addition to stopping at tag openers, this will allow
stopping at all tag tokens regardless of open or close status.
@dmsnell dmsnell force-pushed the tag-processor/stop-at-tag-closers branch from 0d78f1d to 7e76592 Compare November 23, 2022 02:17
@dmsnell dmsnell merged commit 7e76592 into trunk Nov 23, 2022
@dmsnell dmsnell deleted the tag-processor/stop-at-tag-closers branch November 23, 2022 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants